[libc++] Fix std::variant evaluating template arguments too eagerly #151028
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
This was referenced Jul 28, 2025
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesFull diff: https://github.com/llvm/llvm-project/pull/151028.diff 2 Files Affected:
diff --git a/libcxx/include/__type_traits/invoke.h b/libcxx/include/__type_traits/invoke.h
index 5ff2efbe5faaf..d7fa979bcc5e7 100644
--- a/libcxx/include/__type_traits/invoke.h
+++ b/libcxx/include/__type_traits/invoke.h
@@ -22,6 +22,7 @@
#include <__type_traits/is_same.h>
#include <__type_traits/is_void.h>
#include <__type_traits/nat.h>
+#include <__type_traits/type_identity.h>
#include <__type_traits/void_t.h>
#include <__utility/declval.h>
#include <__utility/forward.h>
@@ -67,20 +68,20 @@ _LIBCPP_BEGIN_NAMESPACE_STD
#if __has_builtin(__builtin_invoke)
-template <class... _Args>
-using __invoke_result_t _LIBCPP_NODEBUG = decltype(__builtin_invoke(std::declval<_Args>()...));
-
template <class, class... _Args>
struct __invoke_result_impl {};
template <class... _Args>
-struct __invoke_result_impl<__void_t<__invoke_result_t<_Args...> >, _Args...> {
- using type _LIBCPP_NODEBUG = __invoke_result_t<_Args...>;
+struct __invoke_result_impl<__void_t<decltype(__builtin_invoke(std::declval<_Args...>()))>, _Args...> {
+ using type _LIBCPP_NODEBUG = decltype(__builtin_invoke(std::declval<_Args...>()));
};
template <class... _Args>
using __invoke_result _LIBCPP_NODEBUG = __invoke_result_impl<void, _Args...>;
+template <class... _Args>
+using __invoke_result_t _LIBCPP_NODEBUG = typename __invoke_result<_Args...>::type;
+
template <class... _Args>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __invoke_result_t<_Args...> __invoke(_Args&&... __args)
_NOEXCEPT_(noexcept(__builtin_invoke(std::forward<_Args>(__args)...))) {
diff --git a/libcxx/test/std/test.pass.cpp b/libcxx/test/std/test.pass.cpp
new file mode 100644
index 0000000000000..1ff28c481e6ee
--- /dev/null
+++ b/libcxx/test/std/test.pass.cpp
@@ -0,0 +1,24 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include <variant>
+#include <utility>
+
+struct Nullable {
+ template <typename V> Nullable(V);
+};
+
+struct Matcher {
+ Matcher();
+ Matcher(std::variant<Nullable>);
+};
+
+void f() {
+ Matcher vec;
+ Matcher m = std::move(vec);
+}
|
philnik777
force-pushed
the
invoke_less_eager_instantiation
branch
from
6153d21 to
10260aa
Compare
philnik777
changed the title
ldionne
reviewed
Jul 31, 2025
libcxx/test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/include/variant
Outdated
| @@ -1182,13 +1182,21 @@ public: | |||
| _LIBCPP_HIDE_FROM_ABI constexpr variant(const variant&) = default; | |||
| _LIBCPP_HIDE_FROM_ABI constexpr variant(variant&&) = default; | |||
|
|
|||
| template < class _Arg, | |||
There was a problem hiding this comment.
I would suggest this instead, which is easier to read IMO:
template <class _Arg, class _DependentArg = enable_if_t<
!is_same_v<__remove_cvref_t<_Arg>, variant> &&
!__is_inplace_type<__remove_cvref_t<_Arg>>::value &&
!__is_inplace_index<__remove_cvref_t<_Arg>>::value,
_Arg
>,
class _Tp = __variant_detail::__best_match_t<_DependentArg, _Types...>,
size_t _Ip = __find_detail::__find_unambiguous_index_sfinae<_Tp, _Types...>::value,
enable_if_t<is_constructible_v<_Tp, _Arg>, int> = 0
>There was a problem hiding this comment.
This doesn't work. I'm going with the original for now and we can take another look later.
ldionne
approved these changes
Jul 31, 2025
github-project-automation
bot
moved this from Needs Triage
to Needs Merge
in LLVM Release Status
philnik777
force-pushed
the
invoke_less_eager_instantiation
branch
3 times, most recently
from
2627e8b to
9c7f246
Compare
philnik777
force-pushed
the
invoke_less_eager_instantiation
branch
from
9c7f246 to
dc2d9e7
Compare
ldionne
approved these changes
Aug 11, 2025
github-project-automation
bot
moved this from Needs Backport PR
to Needs Merge
in LLVM Release Status
github-project-automation
bot
moved this from Needs Merge
to Done
in LLVM Release Status
|
/cherry-pick f5f5824 |
|
/pull-request #153064 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This has been reported in #116709 (comment).
Fixes #151328